dao/src/main/java/greencity/repository/NotificationRepo.java (1)

195-195: Add Javadoc for new repository method.

The new countByTimeAfter method follows Spring Data JPA naming conventions and will work correctly. To maintain consistency with the rest of the interface, please add Javadoc documentation for this method.

+/**
+ * Counts the number of notifications created after the specified date.
+ *
+ * @param date the date after which to count notifications
+ * @return the count of notifications created after the specified date
+ */
 long countByTimeAfter(ZonedDateTime date);

core/src/main/resources/application.properties (1)

104-109: Consider limiting exposed actuator endpoints.

The Prometheus configuration looks good, but exposing all management endpoints (*) could potentially expose sensitive information or operations. Consider restricting this to only the endpoints you actually need.

#Prometheus
-management.endpoints.web.exposure.include=*
+management.endpoints.web.exposure.include=health,info,prometheus,metrics
#Optional path
management.endpoints.web.base-path=/actuator
#Also optional
management.endpoint.health.show-details=always

core/src/test/java/greencity/controller/MetricsControllerTest.java (1)

46-83: Great edge case coverage, consider adding exception handling tests.

These tests do an excellent job covering various null and empty input scenarios. However, I recommend adding tests for exception handling scenarios to ensure robustness.

Consider adding a test like this:

@Test
void recordLoginWhenServiceThrowsExceptionTest() {
    // Given
    doThrow(new RuntimeException("Service failure")).when(activeUsersInMemoryMetrics)
        .recordLogin("error@example.com", 1234567890L);
    
    LoginEventDto errorDto = new LoginEventDto("error@example.com", 1234567890L);
    
    // When/Then
    assertThrows(RuntimeException.class, () -> metricsController.recordLogin(errorDto));
    
    verify(activeUsersInMemoryMetrics).recordLogin("error@example.com", 1234567890L);
    verifyNoMoreInteractions(activeUsersInMemoryMetrics);
}

core/src/main/java/greencity/dto/metric/LoginEventDto.java (1)

7-13: Consider adding validation annotations to the fields.

This DTO looks well-structured with appropriate Lombok annotations. To enhance robustness, consider adding validation annotations like @Email for the email field and @NotNull for both fields. Also, adding documentation to clarify the expected format of loginTime (e.g., milliseconds since epoch) would improve maintainability.

+import javax.validation.constraints.Email;
+import javax.validation.constraints.NotNull;

 @Data
 @AllArgsConstructor
 @NoArgsConstructor
 public class LoginEventDto {
+    @NotNull
+    @Email
     private String email;
+    @NotNull
     private Long loginTime;
 }

service/src/main/java/greencity/health/MemoryUsageHealthIndicator.java (1)

18-18: Make memory threshold configurable.

Consider making the memory usage threshold configurable through a property rather than hardcoding it. This allows for environment-specific tuning without code changes.

-    private static final double MAX_MEMORY_USAGE_PERCENTAGE = 80.0;
+    @Value("${health.memory.max-usage-percentage:80.0}")
+    private double maxMemoryUsagePercentage;

Don't forget to add the import:

import org.springframework.beans.factory.annotation.Value;

service/src/main/java/greencity/health/DatabasePerformanceHealthIndicator.java (1)

36-37: Make database type configurable rather than hardcoded

The health indicator hardcodes "PostgreSQL" as the database type, which may not be accurate if a different database is used. Consider making this configurable or determining it dynamically.

+   private String determineDatabaseType(Connection connection) {
+       try {
+           return connection.getMetaData().getDatabaseProductName();
+       } catch (Exception e) {
+           logger.warn("Could not determine database type: {}", e.getMessage());
+           return "Database";
+       }
+   }

@Override
public Health health() {
    try (Connection connection = dataSource.getConnection();
        Statement statement = connection.createStatement()) {
        long startTime = System.currentTimeMillis();
        statement.execute("SELECT 1");
        long latencyMs = System.currentTimeMillis() - startTime;

        meterRegistry.gauge("app_database_health", 1);
        return Health.up()
-           .withDetail("database", "PostgreSQL is reachable")
+           .withDetail("database", determineDatabaseType(connection) + " is reachable")
            .withDetail("latencyMs", latencyMs)
            .build();

core/src/main/java/greencity/metrics/CPUMetrics.java (1)

3-4: Consider adding a note about the use of com.sun.management package

The com.sun.management package is an Oracle-specific API that might not be available in all JVM implementations, which could be a portability concern. Consider adding a comment to document this dependency and potential alternatives.

+/**
+ * Monitors CPU usage metrics using Micrometer.
+ * Note: This class depends on com.sun.management.OperatingSystemMXBean which is
+ * an implementation-specific API that might not be available in all JVMs.
+ */
@Component
public class CPUMetrics {

service/src/test/java/greencity/health/UserEngagementHealthIndicatorTest.java (1)

41-43: Mocking gauge() is redundant and may conceal signature mismatches

MeterRegistry#gauge(String, Number) returns the same Number instance back and its return value isn’t used inside health().
You can safely drop the stubbing lines and keep only the verify step. This trims noise and avoids a brittle eq(1) / eq(0) matcher that might break if the production code switches to 1.0 (Double) or tags.

-when(meterRegistry.gauge(eq("app_user_engagement_health"), eq(1))).thenReturn(null);
+// no stubbing needed – we only verify the invocation later

(and similarly for eq(0))

Also applies to: 66-68, 92-94

service/src/main/java/greencity/health/DiskSpaceHealthIndicator.java (1)

36-45: Consider re-using a single Gauge instead of calling gauge() every check

meterRegistry.gauge("app_disk_space_health", value) creates a new gauge each invocation, which can leak meters over time. Prefer registering once in the constructor (as done in ResponseTimeMetrics) or by updating a AtomicInteger.

core/src/main/java/greencity/metrics/ResponseTimeMetrics.java (1)

66-71: Null-check guard adds no value inside a servlet container

OncePerRequestFilter is only invoked with non-null request, response, and filterChain; the explicit null guard adds overhead without providing extra safety. Consider removing validateParameters() to streamline the hot path.

core/src/test/java/greencity/metrics/MemoryMetricsTest.java (2)

69-80: Consider extracting reflection utility methods.

The reflection code for accessing private fields appears in multiple places throughout the test class. Consider extracting these operations into utility methods to improve readability and reduce duplication.

+ private <T> T getPrivateField(Object object, String fieldName, Class<T> fieldType) throws NoSuchFieldException, IllegalAccessException {
+     Field field = object.getClass().getDeclaredField(fieldName);
+     field.setAccessible(true);
+     return fieldType.cast(field.get(object));
+ }

---

82-94: Verify gauge updates properly.

The test correctly verifies that the gauges are registered with the meter registry. However, it only checks that the gauges are registered, not that they return the expected values. Consider enhancing this test to verify the actual values returned by the gauges.

service/src/main/java/greencity/health/NotificationServiceHealthIndicator.java (1)

34-36: Consider using UTC for time calculations.

Using ZoneId.systemDefault() can lead to inconsistent behavior when deploying across different environments with different time zones. Consider using UTC for more consistent time-based calculations.

-            LocalDateTime last24HoursLocal = LocalDateTime.now().minusHours(24);
-            ZonedDateTime last24Hours = last24HoursLocal.atZone(ZoneId.systemDefault());
+            ZonedDateTime now = ZonedDateTime.now(ZoneId.of("UTC"));
+            ZonedDateTime last24Hours = now.minusHours(24);

service/src/test/java/greencity/health/DiskSpaceHealthIndicatorTest.java (1)

56-76: Add test for threshold boundary values.

The test cases cover scenarios well above and well below the threshold, but consider adding a test at exactly the threshold boundary to ensure that edge cases are handled correctly.

For a complete test suite, add a test case for:

• Free space exactly at the 10% threshold (100L free space with 1000L total)
• Free space just below the threshold (99L free space with 1000L total)
• Free space just above the threshold (101L free space with 1000L total)

core/src/main/java/greencity/metrics/RequestCountMetrics.java (3)

74-86: Consider adding logging for request ID generation.

When generating new request IDs, consider adding debug logging to help with troubleshooting in production environments.

 private String ensureRequestId(HttpServletRequest request, HttpServletResponse response) {
     String requestId = request.getHeader(REQUEST_ID_HEADER);
     if (requestId == null || requestId.isEmpty()) {
         requestId = UUID.randomUUID().toString();
         response.setHeader(REQUEST_ID_HEADER, requestId);
         request.setAttribute(REQUEST_ID_HEADER, requestId);
+        logger.debug("Generated new request ID: {}", requestId);
     }
     return requestId;
 }

---

155-159: Add safeguard against potential division by zero.

While the code checks if total is 0, it's good practice to make this more explicit and handle potential arithmetic exceptions.

 private double getErrorRate() {
     int total = totalRequests.getCount();
     int failed = failedRequests.getCount();
-    return total == 0 ? 0.0 : (double) failed / total * 100;
+    return total > 0 ? Math.min((double) failed / total * 100, 100.0) : 0.0;
 }

This change also caps the error rate at 100% as a safeguard against potential inconsistencies in the counters.

---

172-176: Consider adding batch operations for metrics updates.

The increment method adds a timestamp and increments the count atomically, but then immediately cleans up old timestamps. For high-traffic scenarios, this could create a lot of overhead.

Consider cleaning up old timestamps periodically rather than on every increment:

 void increment(Instant timestamp) {
     timestamps.add(timestamp);
     count.incrementAndGet();
-    cleanupOldTimestamps(timestamp);
+    // Cleanup old timestamps periodically instead of on every increment
+    if (timestamps.size() % 100 == 0) {
+        cleanupOldTimestamps(timestamp);
+    }
 }

service/src/main/java/greencity/health/HabitTrackingHealthIndicator.java (2)

53-70: Consider refactoring duplicate health response building logic

The code for building health responses has similar patterns in both the success and failure paths. Consider extracting a helper method to build the health response with common details, which would reduce duplication and make future maintenance easier.

@Override
public Health health() {
    try {
        LocalDateTime last24Hours = LocalDateTime.now().minusHours(24);
        ZonedDateTime zonedLast24Hours = last24Hours.atZone(ZoneId.systemDefault());

        Query newHabitsQuery = entityManager.createNativeQuery(
            "SELECT COUNT(*) FROM habits WHERE created_at >= :last24Hours");
        newHabitsQuery.setParameter("last24Hours", last24Hours);
        long newHabitsCount = ((Number) newHabitsQuery.getSingleResult()).longValue();

        Query newAssignsQuery = entityManager.createNativeQuery(
            "SELECT COUNT(*) FROM habit_assign WHERE create_date >= :last24Hours");
        newAssignsQuery.setParameter("last24Hours", zonedLast24Hours);
        long newAssignsCount = ((Number) newAssignsQuery.getSingleResult()).longValue();

        long totalActivityCount = newHabitsCount + newAssignsCount;

-       if (totalActivityCount >= minHabitUpdates) {
-           meterRegistry.gauge("app_habit_tracking_health", 1);
-           return Health.up()
-               .withDetail("habitTracking", "Habit tracking is active")
-               .withDetail("newHabitsLast24h", newHabitsCount)
-               .withDetail("newAssignsLast24h", newAssignsCount)
-               .withDetail("totalActivityLast24h", totalActivityCount)
-               .build();
-       } else {
-           meterRegistry.gauge("app_habit_tracking_health", 0);
-           return Health.outOfService()
-               .withDetail("habitTracking", "Low habit tracking activity")
-               .withDetail("newHabitsLast24h", newHabitsCount)
-               .withDetail("newAssignsLast24h", newAssignsCount)
-               .withDetail("totalActivityLast24h", totalActivityCount)
-               .withDetail("minThreshold", minHabitUpdates)
-               .build();
-       }
+       boolean isHealthy = totalActivityCount >= minHabitUpdates;
+       meterRegistry.gauge("app_habit_tracking_health", isHealthy ? 1 : 0);
+       
+       Health.Builder builder = isHealthy ? 
+           Health.up().withDetail("habitTracking", "Habit tracking is active") :
+           Health.outOfService().withDetail("habitTracking", "Low habit tracking activity");
+           
+       builder.withDetail("newHabitsLast24h", newHabitsCount)
+              .withDetail("newAssignsLast24h", newAssignsCount)
+              .withDetail("totalActivityLast24h", totalActivityCount);
+              
+       if (!isHealthy) {
+           builder.withDetail("minThreshold", minHabitUpdates);
+       }
+       
+       return builder.build();
    } catch (Exception e) {
        logger.error("Habit tracking health check failed: {}", e.getMessage());
        meterRegistry.gauge("app_habit_tracking_health", 0);
        return Health.down()
            .withDetail("habitTracking", "Error checking habit tracking")
            .withDetail("error", e.getMessage())
            .build();
    }
}

---

41-44: Consider using JPQL instead of native SQL queries

Using native SQL queries couples your code to the specific database schema. JPQL would provide better maintainability and database independence.

For example:

Query newHabitsQuery = entityManager.createQuery(
    "SELECT COUNT(h) FROM Habit h WHERE h.createdAt >= :last24Hours");

core/src/main/java/greencity/metrics/ActiveUsersInMemoryMetrics.java (1)

83-85: Reconsider the login count calculation method

The current implementation of getUserLoginsCount() sums the sizes of all login timestamp lists, which counts repeated logins from the same user multiple times. Consider if you want unique user login count instead.

private int getUserLoginsCount() {
-   return userLogins.values().stream().mapToInt(List::size).sum();
+   // If you want unique user logins:
+   return userLogins.size();
+   // Or if you want total logins but need to handle empty lists:
+   // return userLogins.values().stream().filter(list -> !list.isEmpty()).mapToInt(List::size).sum();
}

core/src/main/java/greencity/config/SecurityConfig.java (1)

131-132: Document filter ordering rationale

The position of filters in the security chain is important. Consider adding comments explaining why ActiveUsersInMemoryMetrics is placed after AccessTokenAuthenticationFilter and why RequestCountMetrics is placed before it.

+ // Place ActiveUsersInMemoryMetrics after auth filter to ensure we only track authenticated users
.addFilterAfter(new ActiveUsersInMemoryMetrics(meterRegistry), AccessTokenAuthenticationFilter.class)
+ // Place RequestCountMetrics before auth filter to capture all requests including authentication failures
.addFilterBefore(requestCountMetrics, AccessTokenAuthenticationFilter.class)

service/src/main/java/greencity/health/UserEngagementHealthIndicator.java (1)

29-49: Consider initializing the gauge once instead of repeatedly.

The gauge initialization in each condition branch could potentially create multiple gauges with the same name or cause memory leaks if called frequently.

Consider initializing the gauge once in the constructor and updating its value in the health method:

@Component
public class UserEngagementHealthIndicator implements HealthIndicator {
    private static final Logger logger = LoggerFactory.getLogger(UserEngagementHealthIndicator.class);
    private final UserRepo userRepo;
    private final MeterRegistry meterRegistry;
    private final int minUserThreshold;
+   private final AtomicInteger healthGauge;

    @Autowired
    public UserEngagementHealthIndicator(UserRepo userRepo, MeterRegistry meterRegistry,
        @Value("${user.engagement.min.threshold:5}") int minUserThreshold) {
        this.userRepo = userRepo;
        this.meterRegistry = meterRegistry;
        this.minUserThreshold = minUserThreshold;
+       this.healthGauge = meterRegistry.gauge("app_user_engagement_health", new AtomicInteger(0));
    }

    @Override
    public Health health() {
        try {
            LocalDateTime last24Hours = LocalDateTime.now().minusHours(24);
            long newUsers = userRepo.countByDateOfRegistrationAfter(last24Hours);

            if (newUsers >= minUserThreshold) {
-               meterRegistry.gauge("app_user_engagement_health", 1);
+               healthGauge.set(1);
                return Health.up()
                    .withDetail("userEngagement", "User engagement is normal")
                    .withDetail("newUsersLast24h", newUsers)
                    .build();
            } else {
-               meterRegistry.gauge("app_user_engagement_health", 0);
+               healthGauge.set(0);
                return Health.outOfService()
                    .withDetail("userEngagement", "Low user engagement detected")
                    .withDetail("newUsersLast24h", newUsers)
                    .withDetail("minThreshold", minUserThreshold)
                    .build();
            }

Note: Don't forget to import java.util.concurrent.atomic.AtomicInteger if you implement this change.

service/src/test/java/greencity/health/MemoryUsageHealthIndicatorTest.java (1)

36-46: Consider improving testability instead of using reflection.

While reflection works for testing, it makes the tests more brittle and harder to maintain.

Consider modifying the MemoryUsageHealthIndicator class to accept an optional MemoryMXBean parameter in the constructor, defaulting to ManagementFactory.getMemoryMXBean() if not provided:

// In MemoryUsageHealthIndicator.java
public class MemoryUsageHealthIndicator implements HealthIndicator {
    private final MeterRegistry meterRegistry;
    private final MemoryMXBean memoryMXBean;

-   public MemoryUsageHealthIndicator(MeterRegistry meterRegistry) {
+   public MemoryUsageHealthIndicator(MeterRegistry meterRegistry, MemoryMXBean memoryMXBean) {
        this.meterRegistry = meterRegistry;
-       this.memoryMXBean = ManagementFactory.getMemoryMXBean();
+       this.memoryMXBean = memoryMXBean;
    }

+   public MemoryUsageHealthIndicator(MeterRegistry meterRegistry) {
+       this(meterRegistry, ManagementFactory.getMemoryMXBean());
+   }
}

// Then in the test:
@BeforeEach
void setUp() {
-   healthIndicator = new MemoryUsageHealthIndicator(meterRegistry);
-   try {
-       java.lang.reflect.Field field = MemoryUsageHealthIndicator.class.getDeclaredField("memoryMXBean");
-       field.setAccessible(true);
-       field.set(healthIndicator, memoryMXBean);
-   } catch (Exception e) {
-       throw new RuntimeException("Failed to set MemoryMXBean for testing", e);
-   }
+   healthIndicator = new MemoryUsageHealthIndicator(meterRegistry, memoryMXBean);
}

core/src/test/java/greencity/metrics/RequestCountMetricsTest.java (1)

118-126: Make gauge-name assertions order-independent to reduce flakiness

Gauge.builder invocations are captured in a list whose order is not guaranteed by Micrometer’s internal registration logic.
Asserting by position (get(0)…get(4)) risks intermittent failures if the production code is refactored but still registers the same five gauges.

Consider asserting on containment instead:

List<String> names = nameCaptor.getAllValues();
assertTrue(names.containsAll(List.of(
        "http_requests_total_per_hour",
        "http_requests_successful_per_hour",
        "http_requests_failed_per_hour",
        "http_requests_prometheus_per_hour",
        "app_error_rate_per_hour")));
assertEquals(5, names.size());

This keeps the test stable while still guaranteeing that all required gauges are registered.

core/src/test/java/greencity/metrics/ActiveUsersInMemoryMetricsTest.java (1)

182-191: Consider providing a package-private helper instead of reflection in tests

setActiveUser manipulates the activeUsers map through reflection.
A small, package-private method (e.g., void putActiveUserForTest(String, Instant)) inside ActiveUsersInMemoryMetrics would:

• Eliminate reflection complexity.
• Preserve encapsulation at the public API level.
• Fail fast at compile-time if the internal representation changes.

Reflection should be the last resort for test setup.

service/src/test/java/greencity/health/HabitTrackingHealthIndicatorTest.java (1)

71-72: Use double literal when verifying gauge values

MeterRegistry#gauge(String, double) expects a double. Verifying with an int works through widening, but Mockito performs strict type matching on the boxed value, which may lead to false-negatives.

-verify(meterRegistry).gauge("app_habit_tracking_health", 1);
+verify(meterRegistry).gauge("app_habit_tracking_health", 1.0);

Apply the same change for the 0 verifications. This tiny tweak prevents brittle test failures.

Also applies to: 94-95, 110-111, 133-134

core/src/main/java/greencity/metrics/GarbageCollectionMetrics.java (2)

58-60: Potential unbounded growth of history queues

A new record is added on every scrape, and only entries older than 1 hour are removed.
If Prometheus scrapes every 5 seconds, each queue can hold up to 3600 / 5 = 720 elements per collector, which is acceptable.
However, for sub-second scrapes or accidental mis-configuration, the queues could grow unbounded.

Consider guarding against pathologically high scrape rates:

private static final int MAX_SAMPLES = 1000; // safety cap
...
if (history.size() > MAX_SAMPLES) {
    history.poll();
}

This defensive cap prevents memory bloat while preserving the 1-hour window for normal scrape intervals.

---

34-48: Minor: unused lambda parameter

Gauge.builder(..., this, metrics -> getGCTimePerHour(gcBean)) supplies a lambda argument metrics that is never used.
You can replace the lambda with a method reference for clarity:

-Gauge.builder("app_gc_time_ms_per_hour", this, metrics -> getGCTimePerHour(gcBean))
+Gauge.builder("app_gc_time_ms_per_hour", this, unused -> getGCTimePerHour(gcBean))

(or simply obj -> …). Purely cosmetic, but it signals intent more clearly.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro